Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

画面横幅が小さいときのチャンネルヘッダーのレイアウトを変更 #4100

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

damin3A3
Copy link
Contributor

@damin3A3 damin3A3 commented Oct 18, 2023

・画面横幅が小さい時にも通知設定アイコンが表示されるようにした
・画面横幅が小さい時にトピックが表示されないようにした

close #3975

@github-actions
Copy link

@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

All modified lines are covered by tests ✅

📢 Thoughts on this report? Let us know!.

@mehm8128
Copy link
Member

コードまだ読んでないですが先に2点だけ:pray:

  1. close https://github.com/traPtitech/traQ_S-UI/issues/3975みたいにしてissueのリンクをPRの説明欄に貼ると、PRがマージされたときに自動でissueがcloseされるようになるので、今からでも説明文を編集してやってみてください!
  2. 多分手元でcat ~/.gitconfigしたときに表示されるnameemailのどちらかもしくは両方が、githubに登録されているユーザー名・メールアドレスと一致していなくて、↓の画像でcommit messageの左にアイコンが表示されなくなってしまっているので、一致するように直すとよさそうです:eyes:
    image

@damin3A3
Copy link
Contributor Author

ありがとうございます、やってみます!

Copy link
Member

@mehm8128 mehm8128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

該当コードをちゃんと探し出せていて良いと思います!2点書いたので修正お願いします:pray:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

すみません、やっぱりこのファイルの変更は一旦消しておいてほしいです:pray:
多分後で別PRで、トピックをチャンネル名の下に表示するような感じにすると思います(デザインチームともうちょっと話し合うとは思いますが)
https://md.trap.jp/8kLQ2b3BRVOS0yB4OnnC3Q#%E3%83%81%E3%83%A3%E3%83%B3%E3%83%8D%E3%83%AB%E3%83%98%E3%83%83%E3%83%80%E3%83%BC%E3%81%AE%E3%83%88%E3%83%94%E3%83%83%E3%82%AF

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ちなみに、もしトピックを消すとするならここではなくて、このコンポーネントを使っているChannelHeader.vue<channel-header-topic :class="$style.topic" :channel-id="channelId" />に直接v-ifをつけるような変更になると思います

Comment on lines 31 to 38
<header-tools-item
:class="$style.notificationIcon"
:data-state="subscriptionChangeInfo.state"
:icon-name="subscriptionChangeInfo.iconName"
:disabled="!subscriptionChangeInfo.canChange"
:tooltip="subscriptionChangeInfo.tooltip"
@click="changeToNextSubscriptionLevel"
/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

今と順番が逆になってしまっているので、順番は同じままにしてほしいです!

↓順に今のPC、変更後のPC、変更後のスマホ
image
image
image

@damin3A3
Copy link
Contributor Author

damin3A3 commented Oct 18, 2023

ありがとうございます!
アイコンの順序については気づいていませんでした、修正します!
トピックの方は元の形に再変更しておきます、すみませんでした。

@@ -50,4 +50,4 @@ const topic = computed(() => channelsMap.value.get(props.channelId)?.topic)
text-overflow: ellipsis;
white-space: nowrap;
}
</style>
</style>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここファイルの最後の改行が消されちゃっているので、戻しておいてほしいです:pray:
VSCodeの設定で
"files.insertFinalNewline": true
というのを入れると保存時に自動で入れてくれると思います

@mehm8128
Copy link
Member

チャンネル名のサイズ後で小さくする前提だったからチャンネル名圧迫されちゃってるのどうしようかな

@damin3A3
Copy link
Contributor Author

ありがとうございます!
コピペした際に改行を消してしまったみたいです、すみません。
これだけ修正してコミットしてしまって大丈夫ですか?
チャンネル名の方も何か問題がありましたか…?

@mehm8128
Copy link
Member

改行についてはそれだけ直してcommitしてもらってOKです!
チャンネル名については、↓の画像が順に変更前、変更後で、単純にボタンを増やすとチャンネル名の幅が狭くなってしまうので、チャンネル名小さくしちゃっていいんじゃない?って話をデザインチームの人と話してたって感じです
image
image

申し訳ないのですが、改行のcommitだけ一旦してもらったらデザインチームの人と結論出すまでマージは待ってほしいです:pray:

@damin3A3
Copy link
Contributor Author

分かりました!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants